Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: add default client configurations #1312

Merged
merged 2 commits into from
Feb 19, 2024
Merged

feat: add default client configurations #1312

merged 2 commits into from
Feb 19, 2024

Conversation

AndrewFossAWS
Copy link
Contributor

@AndrewFossAWS AndrewFossAWS commented Jan 22, 2024 •
edited
Loading

Description of changes

  • See feat: add runtime plugins smithy-lang/smithy-swift#650 for context on client configurations
  • Added AWS specific default client configurations
  • AWSClientConfigDefaultsProvider.swift provides all the default values for client config properties
  • Code generate AWS clients configurations (align with Smithy SRA runtime-plugins design)

New/existing dependencies impact assessment, if applicable

Conventional Commits

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@AndrewFossAWS AndrewFossAWS changed the title feat: add runtime plugins feat: add default client configurations Jan 22, 2024
@AndrewFossAWS AndrewFossAWS marked this pull request as ready for review January 22, 2024 21:32
@jbelkins jbelkins self-requested a review January 25, 2024 17:19
dayaffe
dayaffe reviewed Feb 19, 2024
View reviewed changes
Copy link
Contributor

@dayaffe dayaffe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just one nit here I'd like addressed! I am good with the code logic

Sources/Core/AWSClientRuntime/AWSClientConfigDefaultsProvider.swift Outdated
Comment on lines 94 to 100
// swiftlint:disable switch_case_alignment
switch try self.retryMode(nil) {
case .legacy, .standard:
resolvedRateLimitingMode = .standard
case .adaptive:
resolvedRateLimitingMode = .adaptive
}
// swiftlint:enable switch_case_alignment

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: should remove switftlint disable switch_case_alignment and move case statements over so that they are in-line with switch

@AndrewFossAWS AndrewFossAWS dismissed jbelkins’s stale review February 19, 2024 17:39

I have addressed Josh's comments

@AndrewFossAWS
Copy link
Contributor Author

/test

@AndrewFossAWS AndrewFossAWS merged commit 7bc88d0 into main Feb 19, 2024
17 checks passed
@AndrewFossAWS AndrewFossAWS deleted the runtime-plugins branch February 19, 2024 18:44
@sichanyoo sichanyoo mentioned this pull request Sep 12, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jbelkins jbelkins jbelkins left review comments

@syall syall syall approved these changes

@dayaffe dayaffe dayaffe approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@AndrewFossAWS @jbelkins @dayaffe @syall